Conversation
jazzz
left a comment
jazzz
left a comment
There was a problem hiding this comment.
I'm not against adding Just to make working with the repository easier to work with - especially as more complex tasks are added.
However this usecase seems unneeded. Examples exist to allow developers to see how to use the library and act as a starting point for exploration. Running all of them often seems counter productive.
Is there a reason I am missing?
| run-examples: | ||
| cargo run --example double_ratchet_basic | ||
| cargo run --example serialization_demo | ||
| cargo run --example storage_demo | ||
| cargo run --example out_of_order_demo |
There was a problem hiding this comment.
Some of these examples are hidden behind feature flags. If we want people to be able to run examples easily then it would make sense to remove the storage feature as it doesn't seem that optional.
There was a problem hiding this comment.
The storage feature has been removed in this PR: #30, please review this PR too when you see fit.
| cargo test | ||
|
|
||
| # Run all examples | ||
| run-examples: |
There was a problem hiding this comment.
Is this actually valuable? How often does someone want to run all the examples at once?
This seems like it is trying to perform the same job as integration tests.
There was a problem hiding this comment.
Yep, I run this command for the PRs I made after this change, before I was running such demos one by one, and easily forgot one and introduce bug.
In my previous experience, I see a lot of failing demos or even no demos in some of logos messaging repos, I want to avoid such scenarios happened in libchat.
There was a problem hiding this comment.
Agreed.
However if the goal is to ensure that demo code isn't broken, then it should be ensured via CI and tests.
Add tests that ensure specific behaviour you are looking for and then add cargo test --examples to ensure they meet the expected results.
Simply executing the examples doesn't ensure they are operating correctly.
There was a problem hiding this comment.
Yes, CI will be added later.
Tests is not clearly show the workflow like demos, that's why most rust libraries contains examples directory, which is good in may ways.
Running examples ensures it successful, without such helper, it will be hard to check all the examples easily.
There was a problem hiding this comment.
I also updated the readme to indicate just is not mandatory, and using Cargo as an option to get started.
2 participants
No description provided.